Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix regression with environment markers handling #3967

Closed
wants to merge 1 commit into from
Closed

fix regression with environment markers handling #3967

wants to merge 1 commit into from

Conversation

benoit-pierre
Copy link
Member

This fix #3781. Turn out the problem was not with the parsing code but how a Requirement is converted back to a string:

In [1]: import pip

In [2]: pip.__version__
Out[2]: '8.1.2'

In [3]: str(pip._vendor.packaging.requirements.Requirement("python-xlib>=0.16; 'linux' in sys_platform"))
Out[3]: 'python-xlib>=0.16; linux in "sys_platform"'

The patch ensure the result of str(Requirement()) is itself a valid requirement line that can be parsed back.

@benoit-pierre
Copy link
Member Author

So the "nightly" job on Travis failed, I don't know why... On my repository build, it also failed, but for some reason the "pypy" job failed too...

Any idea why?

@benoit-pierre
fix regression with environment markers handling
e49fb2b 
Ensure the result of str(Requirement()) is itself a valid
requirement line that can be parsed back.
@benoit-pierre benoit-pierre force-pushed the fix_requirement_markers_regression branch from 2cea810 to e49fb2b Compare September 15, 2016 19:27
@benoit-pierre
Copy link
Member Author

Found an issue when porting the patch to setuptools codebase, so I amended the code and added more tests.

@dstufft
Copy link
Member

dstufft commented Sep 15, 2016

This needs to be made to https://github.com/pypa/packaging not to pip itself.

@benoit-pierre
Copy link
Member Author

@dstufft: OK, did not know about packaging, which actually already works fine thanks to this merged PR.

@dstufft
Copy link
Member

dstufft commented Sep 15, 2016

Ah yes. I might just need to either cut a new release of packaging or just pull an updated version into pip.

Sent from my iPhone

On Sep 15, 2016, at 4:18 PM, Benoit Pierre [email protected] wrote:

@dstufft: OK, did not know about packaging, which actually already works fine thanks to this merged PR.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@dstufft
Copy link
Member

dstufft commented Sep 15, 2016

Going to close this, will get pulled in via packaging.

@dstufft dstufft closed this Sep 15, 2016
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

InvalidRequirement - possible requirement parsing regression in 8.1.2
2 participants
@benoit-pierre @dstufft